Conversation
…71. Includes support for the new Chainlink oracle messages.
📝 WalkthroughWalkthroughThis PR adds Chainlink Data Streams support to the oracle (messages, protos, params, verification hooks), expands Exchange v2 (derivative matching, spot matching, VWAP, fee-discounts, many new message types and validators), adjusts permissions/permissions protos for wasm/evm hooks, updates EVM precompile bindings (Failure events), and refactors EVM log decoding and various small APIs. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Pull request overview
The pull request adds support for Chainlink Data Streams oracle messages to the Injective chain. This integration extends the existing oracle module to handle a new oracle type alongside existing providers like Pyth and Stork.
Key changes:
- Introduces
ChainlinkDataStreamsoracle type with price state management - Adds RPC methods for relaying Chainlink prices and querying price states
- Implements permissions module enhancements to support separate WASM and EVM contract hooks
- Updates market parameters to support minimal protocol fee disabling
- Adds validation for Chainlink verifier proxy contract addresses
Reviewed changes
Copilot reviewed 61 out of 63 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| proto/injective/oracle/v1beta1/* | Defines Chainlink Data Streams messages, events, and query endpoints |
| proto/injective/permissions/v1beta1/* | Separates contract hooks into wasm_hook and evm_hook fields |
| proto/injective/exchange/v2/* | Adds has_disabled_minimal_protocol_fee field to market proposals |
| chain/oracle/types/params.go | Adds Chainlink verifier proxy contract validation |
| chain/oracle/types/oracle.go | Registers ChainlinkDataStreams oracle type |
| go.mod | Downgrades cometbft dependency version |
| exchange//pb/.go | Generated protobuf code updates |
Comments suppressed due to low confidence (6)
proto/injective/oracle/v1beta1/tx.proto:1
- The comment incorrectly states this message is for updating Pyth prices, but the message is named
MsgRelayChainlinkPrices. Update the comment to reference Chainlink Data Streams instead of Pyth.
proto/injective/oracle/v1beta1/query.proto:1 - Missing space between 'Streams' and 'price' in the comment.
go.mod:1 - The cometbft dependency is being downgraded from v1.0.1-inj.4 to v1.0.1-inj.3. Verify this version downgrade is intentional and won't introduce compatibility issues or missing features needed by this PR.
proto/injective/exchange/v2/proposal.proto:1 - The enum name
DisableMinimalProtocolFeeUpdateis confusing because the valuesFalseandTruesuggest a boolean state rather than an update operation. Consider renaming toMinimalProtocolFeeStatusor using more descriptive value names likeKEEP_CURRENT,ENABLE,DISABLE.
proto/injective/exchange/v2/market.proto:1 - The amino.oneof_name values include the full package prefix. Verify this is the intended format for amino encoding, as the previous version used simpler names like 'uncapped' and 'capped'. This change could affect serialization compatibility.
proto/injective/exchange/v2/market.proto:1 - The deprecation comment provides good context. Ensure migration guides or release notes document this breaking change and guide users to update their code to use the new fields.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
proto/injective/permissions/v1beta1/params.proto (1)
15-16: Consider naming consistency and gas limit granularity.Two observations:
Naming inconsistency: This field is named
contract_hook_max_gas, whilepermissions.protoandtx.protonow use explicitwasm_hook/evm_hooknaming. Consider whether this should be renamed for consistency (e.g., to a more generic name) or if separate limits are needed.Single gas limit: With distinct WASM and EVM hooks now supported, verify whether a single shared gas limit is intentional. EVM and CosmWasm have different gas semantics, so separate limits (
wasm_hook_max_gas/evm_hook_max_gas) might provide finer control.Makefile (1)
7-7: Verify that the feature branch reference is intentional.The
clone-injective-coretarget now references a feature branch (c-655/add_chainlink_data_streams_oracle) instead of a tagged release. While this is appropriate during development, consider:
- Before merging this PR, the core dependency should likely point to a stable release tag
- Feature branches can be rebased or force-pushed, leading to non-reproducible builds
- If this is intentional for coordinated development across repositories, document the dependency relationship
chain/exchange/types/v2/subaccount.go (1)
77-85: Consider adding nil guard for defensive coding.
AddSubaccountOrderassumesdandd.Orderare non-nil. If called with a nil argument, this will panic. If callers are guaranteed to pass valid data, this is fine; otherwise, consider adding a nil check.🔎 Optional defensive nil check
func (r *SubaccountOrderResults) AddSubaccountOrder(d *SubaccountOrderData) { + if d == nil || d.Order == nil { + return + } if d.Order.IsReduceOnly { r.ReduceOnlyOrders = append(r.ReduceOnlyOrders, d)chain/exchange/types/v2/spot.go (2)
22-35: Silent fallthrough for unknownOrderFillTypemay mask bugs.The default return of
TransientSellOrderbookFillsfor unknown fill types could hide programming errors. Consider returningnilto make unhandled cases explicit, or panic if this represents an invariant violation.🔎 Suggested fix
func (r *SpotOrderbookMatchingResults) GetOrderbookFills(fillType OrderFillType) *OrderbookFills { switch fillType { case RestingLimitBuy: return r.RestingBuyOrderbookFills case RestingLimitSell: return r.RestingSellOrderbookFills case TransientLimitBuy: return r.TransientBuyOrderbookFills case TransientLimitSell: return r.TransientSellOrderbookFills + default: + return nil } - - return r.TransientSellOrderbookFills }
228-240: Minor: simplify variable declaration.The
existingVwapDatadeclaration and immediate assignment can be combined.🔎 Simplified variable initialization
func (p *SpotVwapInfo) ApplyVwap(marketID common.Hash, newVwapData *SpotVwapData) { - var existingVwapData *SpotVwapData - - existingVwapData = (*p)[marketID] + existingVwapData := (*p)[marketID] if existingVwapData == nil { existingVwapData = NewSpotVwapData() (*p)[marketID] = existingVwapData }chain/exchange/types/v2/fee_discounts.go (1)
274-321: Incorrect slice capacity and inefficient sort comparison.
Line 278 uses
len(info.AccountFeeTiers)as capacity forsubaccountVolumes, but iterates over the nestedSubaccountMarketVolumeContributionsmap.Lines 301-305 use
appendinside the sort comparison function, which creates temporary allocations on every comparison call. Consider pre-computing the concatenated keys.🔎 Proposed optimization
func (info *FeeDiscountStakingInfo) GetSortedSubaccountAndMarketVolumes() ( []*SubaccountVolumeContribution, []*MarketVolumeContribution, ) { - subaccountVolumes := make([]*SubaccountVolumeContribution, 0, len(info.AccountFeeTiers)) + subaccountVolumes := make([]*SubaccountVolumeContribution, 0) marketVolumeTracker := make(map[common.Hash]VolumeRecord) // ... iteration code unchanged ... info.AccountVolumesMux.RUnlock() + // Pre-compute sort keys to avoid allocations in comparison + type sortKey struct { + idx int + key []byte + } + keys := make([]sortKey, len(subaccountVolumes)) + for i, sv := range subaccountVolumes { + keys[i] = sortKey{idx: i, key: append(sv.SubaccountID.Bytes(), sv.MarketID.Bytes()...)} + } + sort.SliceStable(keys, func(i, j int) bool { + return bytes.Compare(keys[i].key, keys[j].key) < 0 + }) + sorted := make([]*SubaccountVolumeContribution, len(subaccountVolumes)) + for i, k := range keys { + sorted[i] = subaccountVolumes[k.idx] + } + subaccountVolumes = sorted - sort.SliceStable(subaccountVolumes, func(i, j int) bool { - return bytes.Compare( - append(subaccountVolumes[i].SubaccountID.Bytes(), subaccountVolumes[i].MarketID.Bytes()...), - append(subaccountVolumes[j].SubaccountID.Bytes(), subaccountVolumes[j].MarketID.Bytes()...), - ) < 0 - })proto/injective/exchange/v2/proposal.proto (1)
82-86: Consider clarifying field naming for readability.The field
has_disabled_minimal_protocol_feeuses a double-negative construction ("disabled" + negative enum values). While functionally correct and likely matching the market type's field name, it can be confusing:
Truemeans "fee is disabled" (no fee)Falsemeans "fee is not disabled" (fee applies)If this naming is intentional to match the market state field, consider adding a brief comment to clarify the semantics. Otherwise, a positive framing like
minimal_protocol_fee_enabledwith inverted enum values might be clearer.Also applies to: 490-493, 590-593
proto/injective/oracle/v1beta1/tx.proto (1)
167-181: Fix stale comment aboveMsgRelayChainlinkPricesThe comment still references Pyth instead of Chainlink, which can confuse readers.
Proposed comment fix
-// MsgRelayPythPrices defines a SDK message for updating Pyth prices +// MsgRelayChainlinkPrices defines a SDK message for relaying Chainlink prices message MsgRelayChainlinkPrices {chain/oracle/types/msgs.go (1)
406-433: MsgRelayChainlinkPrices wiring mirrors existing Pyth relay semantics
Route,Type,GetSignBytes, andGetSignersare implemented consistently with the other oracle messages, andValidateBasiccorrectly enforces a valid bech32 sender address.If the handler relies on at least one
reportsentry, you may optionally enforcelen(msg.Reports) > 0here to fail fast on obviously empty relays; otherwise, current behaviour matches the Pyth relay pattern.chain/exchange/types/v2/msgs.go (1)
965-973: Centralized validation for market orders; post‑only now rejected for market messagesThe new helpers and their wiring introduce a clear, consistent validation policy:
ValidateSpotMarketOrder/ValidateDerivativeMarketOrder/ValidateBinaryOptionsMarketOrder:
- Reject
BUY_PO/SELL_POfor all market orders, which aligns with “post‑only” being a limit‑order concept.- For binary options, additionally reject conditional types for market orders and reuse the existing binary band validation via
order.ValidateBasic(senderAddr, true).MsgCreateSpotMarketOrder,MsgCreateDerivativeMarketOrder, andMsgCreateBinaryOptionsMarketOrdernow delegate to these helpers.MsgBatchUpdateOrders.ValidateBasicnow applies the same helpers toSpotMarketOrdersToCreate,DerivativeMarketOrdersToCreate, andBinaryOptionsMarketOrdersToCreate, ensuring single and batched flows share identical rules.Net effect: any client that previously sent post‑only market orders will now receive
ErrInvalidOrderTypeForMessage. That looks intentional but is a behavioural change worth being aware of.As a minor defensive improvement, you could also nil‑check the
orderpointer in each helper before dereferencing, in case a malformed batch ever contained a nil entry.Also applies to: 2110-2126, 2727-2752
chain/exchange/types/v2/derivative.go (2)
906-908: Remove commented-out debug code.This commented-out debug statement should be removed before merging.
🔎 Proposed fix
if position.Position != nil { positions = append(positions, position.Position) nonNilPositionSubaccountIDs = append(nonNilPositionSubaccountIDs, subaccountID) } - - // else { - // fmt.Println("❌ position is nil for subaccount", subaccountID.Hex()) - // } }
134-146: Consider documenting the nil position indicator pattern.
SetPositionsilently returns whenpositionis nil, whileSetPositionIndicatorexplicitly sets a nil position. This asymmetry could lead to confusion. Consider adding a brief comment explaining the intended usage pattern for position indicators.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
chain/exchange/types/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/query.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/market.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/proposal.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/query.pb.gois excluded by!**/*.pb.gochain/oracle/types/events.pb.gois excluded by!**/*.pb.gochain/oracle/types/genesis.pb.gois excluded by!**/*.pb.gochain/oracle/types/oracle.pb.gois excluded by!**/*.pb.gochain/oracle/types/query.pb.gois excluded by!**/*.pb.gochain/oracle/types/tx.pb.gois excluded by!**/*.pb.gochain/permissions/types/params.pb.gois excluded by!**/*.pb.gochain/permissions/types/permissions.pb.gois excluded by!**/*.pb.gochain/permissions/types/tx.pb.gois excluded by!**/*.pb.goexchange/auction_rpc/pb/goadesign_goagen_injective_auction_rpc.pb.gois excluded by!**/*.pb.goexchange/auction_rpc/pb/goadesign_goagen_injective_auction_rpc_grpc.pb.gois excluded by!**/*.pb.goexchange/derivative_exchange_rpc/pb/goadesign_goagen_injective_derivative_exchange_rpc.pb.gois excluded by!**/*.pb.goexchange/event_provider_api/pb/goadesign_goagen_event_provider_api.pb.gois excluded by!**/*.pb.goexchange/oracle_rpc/pb/goadesign_goagen_injective_oracle_rpc.pb.gois excluded by!**/*.pb.goexchange/portfolio_rpc/pb/goadesign_goagen_injective_portfolio_rpc.pb.gois excluded by!**/*.pb.gogo.sumis excluded by!**/*.sum
📒 Files selected for processing (43)
Makefilechain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.gochain/evm/precompiles/bank/i_bank_module.abigen.gochain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.gochain/evm/precompiles/exchange/i_exchange_module.abigen.gochain/evm/precompiles/staking/i_staking_module.abigen.gochain/evm/types/events.gochain/evm/types/utils.gochain/exchange/types/codec.gochain/exchange/types/errors.gochain/exchange/types/expected_keepers.gochain/exchange/types/msgs.gochain/exchange/types/proposal.gochain/exchange/types/v2/authz_exchange_generic.gochain/exchange/types/v2/codec.gochain/exchange/types/v2/derivative.gochain/exchange/types/v2/fee_discounts.gochain/exchange/types/v2/market.gochain/exchange/types/v2/msgs.gochain/exchange/types/v2/proposal.gochain/exchange/types/v2/spot.gochain/exchange/types/v2/subaccount.gochain/exchange/types/v2/trade.gochain/oracle/types/errors.gochain/oracle/types/msgs.gochain/oracle/types/oracle.gochain/oracle/types/params.gogo.modinjective_data/chain_messages_list.jsoninjective_data/ofac.jsonproto/injective/exchange/v1beta1/exchange.protoproto/injective/exchange/v1beta1/query.protoproto/injective/exchange/v2/market.protoproto/injective/exchange/v2/proposal.protoproto/injective/exchange/v2/query.protoproto/injective/oracle/v1beta1/events.protoproto/injective/oracle/v1beta1/genesis.protoproto/injective/oracle/v1beta1/oracle.protoproto/injective/oracle/v1beta1/query.protoproto/injective/oracle/v1beta1/tx.protoproto/injective/permissions/v1beta1/params.protoproto/injective/permissions/v1beta1/permissions.protoproto/injective/permissions/v1beta1/tx.proto
🧰 Additional context used
🧬 Code graph analysis (16)
chain/oracle/types/errors.go (1)
chain/oracle/types/key.go (1)
ModuleName(12-12)
chain/exchange/types/msgs.go (1)
chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidStakeGrant(110-110)
chain/exchange/types/v2/trade.go (1)
chain/exchange/types/trading_rewards.go (1)
TradingRewardPoints(9-9)
chain/exchange/types/errors.go (6)
chain/exchange/types/key.go (1)
ModuleName(17-17)chain/peggy/types/key.go (1)
ModuleName(10-10)chain/permissions/types/codec.go (1)
ModuleName(12-12)chain/oracle/types/key.go (1)
ModuleName(12-12)chain/wasmx/types/key.go (1)
ModuleName(10-10)chain/ocr/types/key.go (1)
ModuleName(12-12)
chain/exchange/types/codec.go (4)
chain/exchange/types/tx.pb.go (15)
MsgDecreasePositionMargin(2899-2911)MsgDecreasePositionMargin(2915-2915)MsgDecreasePositionMargin(2916-2918)MsgLiquidatePosition(2573-2582)MsgLiquidatePosition(2586-2586)MsgLiquidatePosition(2587-2589)MsgEmergencySettleMarket(2683-2690)MsgEmergencySettleMarket(2694-2694)MsgEmergencySettleMarket(2695-2697)MsgAuthorizeStakeGrants(3504-3507)MsgAuthorizeStakeGrants(3511-3511)MsgAuthorizeStakeGrants(3512-3514)MsgActivateStakeGrant(3593-3596)MsgActivateStakeGrant(3600-3600)MsgActivateStakeGrant(3601-3603)chain/exchange/types/v2/tx.pb.go (15)
MsgDecreasePositionMargin(3050-3062)MsgDecreasePositionMargin(3066-3066)MsgDecreasePositionMargin(3067-3069)MsgLiquidatePosition(2617-2626)MsgLiquidatePosition(2630-2630)MsgLiquidatePosition(2631-2633)MsgEmergencySettleMarket(2834-2841)MsgEmergencySettleMarket(2845-2845)MsgEmergencySettleMarket(2846-2848)MsgAuthorizeStakeGrants(3658-3663)MsgAuthorizeStakeGrants(3667-3667)MsgAuthorizeStakeGrants(3668-3670)MsgActivateStakeGrant(3749-3754)MsgActivateStakeGrant(3758-3758)MsgActivateStakeGrant(3759-3761)chain/exchange/types/v2/market.pb.go (4)
OpenNotionalCap_Uncapped(109-111)OpenNotionalCap_Uncapped(116-116)OpenNotionalCap_Capped(112-114)OpenNotionalCap_Capped(117-117)chain/exchange/types/v2/query.pb.go (4)
FullDerivativeMarket_PerpetualInfo(3321-3323)FullDerivativeMarket_PerpetualInfo(3328-3328)FullDerivativeMarket_FuturesInfo(3324-3326)FullDerivativeMarket_FuturesInfo(3329-3329)
chain/exchange/types/expected_keepers.go (2)
chain/oracle/types/oracle.pb.go (5)
OracleType(32-32)OracleType(89-91)PriceRecords(1304-1308)PriceRecords(1312-1312)PriceRecords(1313-1315)chain/oracle/types/query.pb.go (6)
ScalingOptions(1604-1607)ScalingOptions(1611-1611)ScalingOptions(1612-1614)PricePairState(1726-1734)PricePairState(1738-1738)PricePairState(1739-1741)
chain/exchange/types/v2/msgs.go (1)
chain/exchange/types/errors.go (4)
ErrInvalidAddress(91-91)ErrInvalidStakeGrant(110-110)ErrInvalidOrderTypeForMessage(68-68)ErrUnrecognizedOrderType(15-15)
chain/exchange/types/v2/subaccount.go (1)
chain/exchange/types/exchange.pb.go (3)
SubaccountOrderData(1812-1815)SubaccountOrderData(1819-1819)SubaccountOrderData(1820-1822)
chain/exchange/types/v2/codec.go (4)
chain/exchange/types/v2/tx.pb.go (12)
MsgDecreasePositionMargin(3050-3062)MsgDecreasePositionMargin(3066-3066)MsgDecreasePositionMargin(3067-3069)MsgEmergencySettleMarket(2834-2841)MsgEmergencySettleMarket(2845-2845)MsgEmergencySettleMarket(2846-2848)MsgAuthorizeStakeGrants(3658-3663)MsgAuthorizeStakeGrants(3667-3667)MsgAuthorizeStakeGrants(3668-3670)MsgActivateStakeGrant(3749-3754)MsgActivateStakeGrant(3758-3758)MsgActivateStakeGrant(3759-3761)chain/exchange/types/v2/proposal.pb.go (3)
DenomMinNotionalProposal(1260-1264)DenomMinNotionalProposal(1268-1268)DenomMinNotionalProposal(1269-1271)chain/exchange/types/v2/market.pb.go (4)
OpenNotionalCap_Uncapped(109-111)OpenNotionalCap_Uncapped(116-116)OpenNotionalCap_Capped(112-114)OpenNotionalCap_Capped(117-117)chain/exchange/types/v2/query.pb.go (4)
FullDerivativeMarket_PerpetualInfo(3321-3323)FullDerivativeMarket_PerpetualInfo(3328-3328)FullDerivativeMarket_FuturesInfo(3324-3326)FullDerivativeMarket_FuturesInfo(3329-3329)
chain/oracle/types/oracle.go (1)
chain/oracle/types/oracle.pb.go (1)
OracleType_ChainlinkDataStreams(48-48)
chain/exchange/types/proposal.go (5)
chain/exchange/types/v2/proposal.pb.go (3)
AdminInfo(614-617)AdminInfo(621-621)AdminInfo(622-624)chain/exchange/types/proposal.pb.go (3)
AdminInfo(566-569)AdminInfo(573-573)AdminInfo(574-576)chain/oracle/types/oracle.pb.go (2)
OracleType_Stork(47-47)OracleType_ChainlinkDataStreams(48-48)chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidPermissions(112-112)chain/exchange/types/market_admin.go (1)
MaxPerm(13-14)
chain/oracle/types/params.go (1)
chain/oracle/types/oracle.pb.go (3)
Params(93-98)Params(102-102)Params(103-105)
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (1)
ethereum/util/contract.go (1)
BoundContract(27-35)
chain/exchange/types/v2/proposal.go (4)
chain/exchange/types/v2/proposal.pb.go (6)
DisableMinimalProtocolFeeUpdate(33-33)DisableMinimalProtocolFeeUpdate(57-59)DisableMinimalProtocolFeeUpdate_NoUpdate(36-36)AdminInfo(614-617)AdminInfo(621-621)AdminInfo(622-624)chain/oracle/types/oracle.pb.go (4)
OracleType_BandIBC(45-45)OracleType_Provider(46-46)OracleType_Stork(47-47)OracleType_ChainlinkDataStreams(48-48)chain/exchange/types/errors.go (2)
ErrInvalidAddress(91-91)ErrInvalidPermissions(112-112)chain/exchange/types/market_admin.go (1)
MaxPerm(13-14)
chain/exchange/types/v2/derivative.go (6)
chain/exchange/types/v2/market.go (2)
DerivativeMarket(211-213)DerivativeMarket(223-225)chain/exchange/types/deposit.go (3)
NewDepositDeltas(42-44)DepositDeltas(40-40)DepositDelta(11-14)chain/exchange/types/trading_rewards.go (2)
NewTradingRewardPoints(11-15)TradingRewardPoints(9-9)chain/exchange/types/market.go (1)
NotionalToChainFormat(130-136)chain/exchange/types/v2/common_utils.go (2)
DerivativeLimitOrderDelta(15-19)DerivativeMarketOrderDelta(21-24)exchange/portfolio_rpc/pb/goadesign_goagen_injective_portfolio_rpc.pb.go (3)
DerivativePosition(635-672)DerivativePosition(687-687)DerivativePosition(702-704)
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (1)
ethereum/util/contract.go (1)
BoundContract(27-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: run-tests
- GitHub Check: lint
🔇 Additional comments (69)
proto/injective/permissions/v1beta1/permissions.proto (1)
13-14: LGTM – EVM hook support added correctly.The field renaming from
contract_hooktowasm_hook(field 2) and addition ofevm_hook(field 8) are well-structured. Field numbers are preserved, ensuring protobuf wire compatibility.Note: Renaming
contract_hook→wasm_hookchanges the JSON field name, which could be a breaking change for clients using JSON encoding. Ensure downstream consumers are updated accordingly.Also applies to: 26-28
proto/injective/permissions/v1beta1/tx.proto (1)
65-77: LGTM – Consistent with permissions.proto changes.The
SetContractHooktype is appropriately reused for bothwasm_hook(field 3) andevm_hook(field 8). Field numbers are preserved, maintaining wire compatibility.Same consideration as noted in
permissions.proto: the JSON field name change fromcontract_hooktowasm_hookmay affect JSON-based clients.go.mod (1)
256-256: No action required. The cometbft version change is part of the coordinated C-655 PR that updates proto definitions to chain v1.18.0 and indexer v1.17.71 and adds Chainlink oracle support, as evidenced by the oracle.proto file changes showing new ChainlinkDataStreams enum and Chainlink-related configuration parameters.chain/oracle/types/errors.go (1)
51-51: LGTM!The new error definition follows the existing error registration pattern and provides a clear message for Chainlink report verification failures. The error code (44) is sequential and appropriate.
chain/evm/precompiles/staking/i_staking_module.abigen.go (1)
40-40: LGTM - Generated code with improved ABI format.The ABI string has been reformatted to use explicit field names (
type,name,internalType), which improves readability while maintaining functional equivalence. Since this is generated code (as indicated by the file header), the change results from regenerating the bindings with an updated toolchain.chain/evm/precompiles/bank/i_bank_module.abigen.go (1)
34-34: LGTM - Generated code with improved ABI format.Consistent with the staking module binding, the ABI string has been reformatted to use explicit field names. This is generated code with improved readability and no functional changes.
chain/exchange/types/errors.go (1)
120-121: LGTM!The two new error definitions follow the established error registration pattern and use sequential error codes (112, 113). The error messages clearly describe the failure conditions related to open notional cap validation and offsetting position requirements.
chain/evm/types/events.go (1)
15-15: No internal references to removed constants found.The constants
EventTypeTxLogandAttributeKeyTxLogare not referenced elsewhere in the codebase, so the removal does not break internal dependencies. If these constants are part of a public API, ensure external consumers are notified of the deprecation or provide migration documentation separately.proto/injective/exchange/v1beta1/exchange.proto (1)
23-28: Update amino.oneof_name changes to use fully qualified names for JSON encoding consistency.The
amino.oneof_nameattributes have been updated to fully qualified names (injective.exchange.v1beta1.OpenNotionalCapUncappedandinjective.exchange.v1beta1.OpenNotionalCapCapped). This improves naming consistency in amino JSON output but does not affect protobuf wire format compatibility (field numbers remain unchanged). No migration strategy is required unless downstream systems explicitly depend on parsing the old short-form strings from amino JSON output.proto/injective/exchange/v2/query.proto (1)
265-265: The HTTP endpoint paths in the v2 proto file show version prefixes consistent with the v2 API structure. However, PositionsInMarket is a new endpoint introduced in V2 (not present in v1beta1), and SubaccountPositions maintained its endpoint structure with only the version prefix updated from v1beta1 to v2. These are not breaking changes to existing REST API endpoints but rather additions and version-specific routing, which is the expected behavior for versioned APIs.Likely an incorrect or invalid review comment.
chain/exchange/types/proposal.go (3)
1655-1665: LGTM! Clean validation implementation for AdminInfo.The
ValidateBasicmethod correctly:
- Validates the Admin address using
sdk.AccAddressFromBech32only when non-empty- Validates
AdminPermissionsagainstMaxPermto ensure no invalid permission bits are set- Uses appropriate error types from the errors registry (
ErrInvalidAddress,ErrInvalidPermissions)
679-686: LGTM! New oracle type correctly added to validation.The
OracleType_ChainlinkDataStreamsis properly included in the valid oracle types switch case, consistent with its definition in the oracle protobuf (OracleType_ChainlinkDataStreams = 13).
290-294: LGTM! Consistent AdminInfo validation integration across proposals.The
AdminInfo.ValidateBasic()call is consistently integrated into all relevant proposal types with proper nil-check and early error return. This centralizes admin validation logic and maintains consistency.Also applies to: 428-432, 535-539, 802-807, 903-908
chain/oracle/types/params.go (3)
55-59: Verify the defaultAcceptUnverifiedChainlinkDataStreamsReports = true.Setting this to
trueby default means unverified Chainlink Data Streams reports will be accepted when no verifier proxy contract is configured. This may be intentional for initial rollout or testing, but please confirm this is the desired security posture for production.
116-132: LGTM! Proper Ethereum address validation.The
ValidateChainlinkVerifierProxyContractfunction correctly:
- Accepts
interface{}for compatibility with param validation patterns- Allows empty string for optional configuration
- Uses
common.IsHexAddressfrom go-ethereum for robust hex address validation
72-81: LGTM! Validation properly extended for new params.The
Validate()method correctly validates both contract addresses with descriptive error wrapping to identify which field failed.chain/evm/types/utils.go (2)
87-97: LGTM! Simplified API with proper bounds checking.The
DecodeMsgLogsfunction now has a cleaner signature and includes proper bounds validation formsgIndex(lines 93-95) to prevent out-of-range access. The function correctly returns an error for invalid indices rather than panicking.
99-110: LGTM! Clean aggregation of logs across all responses.The
DecodeTxLogsfunction correctly iterates through all transaction responses and aggregates logs using thelogsFromTxResponsehelper, maintaining consistency with the single-message variant.proto/injective/oracle/v1beta1/oracle.proto (3)
157-163: Verify intentional type difference forfeed_idbetween messages.
ChainlinkDataStreamsPriceState.feed_idis defined asstring(line 158), whileChainlinkReport.feed_idis defined asbytes(line 314).This may be intentional (e.g.,
ChainlinkReportholds raw data from the report whileChainlinkDataStreamsPriceStatestores a hex-encoded or readable version), but please confirm this is the intended design.Also applies to: 313-318
11-19: LGTM! Params extended with proper field numbering.The new Chainlink-related fields are added with sequential field numbers (2, 3, 4), maintaining protobuf wire compatibility. The field types are appropriate:
stringfor the contract addressboolfor the feature flaguint64for the gas limit
21-36: LGTM! New oracle type properly added to enum.
ChainlinkDataStreams = 13is correctly added following the existing sequence and maintains backward compatibility with existing enum values.chain/exchange/types/expected_keepers.go (1)
31-59: Breaking interface changes require all external implementations to be updated.The
OracleKeeperinterface in this SDK has breaking changes that affect any code implementing these methods:
GetPricePairStatenow requiresscalingOptions *oracletypes.ScalingOptionsparameterGetCumulativePricereturns two values:(baseCumulative, quoteCumulative *sdkmath.LegacyDec)GetHistoricalPriceRecordsreturns(entry *oracletypes.PriceRecords, omitted bool)GetMixedHistoricalPriceRecordsreturns(mixed *oracletypes.PriceRecords, ok bool)Any consumer of this SDK that implements the OracleKeeper interface must update their implementations to match these signatures.
chain/oracle/types/oracle.go (1)
40-41: LGTM!The new
chainlinkdatastreamscase follows the established pattern of other oracle types in the switch statement.proto/injective/oracle/v1beta1/genesis.proto (1)
45-47: LGTM!The new
chainlink_data_streams_price_statesfield follows the established pattern and uses the next sequential field number.chain/exchange/types/v2/subaccount.go (1)
47-67: LGTM!The constructor functions properly initialize all
LegacyDecfields to zero and pre-allocate slices, following good initialization patterns.proto/injective/exchange/v2/market.proto (3)
335-344: Well-documented deprecation.The deprecation includes clear rationale and points to the new replacement fields. Retaining the field number ensures backward compatibility with existing data.
351-364: LGTM!The new
expiration_twap_start_base_cumulative_priceandexpiration_twap_start_quote_cumulative_pricefields properly address the limitation of the deprecated field by tracking base and quote cumulative prices separately.
109-111: Consistent addition ofhas_disabled_minimal_protocol_feeacross market types.The field is added uniformly to
SpotMarket,BinaryOptionsMarket, andDerivativeMarketwith consistent naming and documentation.Also applies to: 187-190, 278-281
chain/exchange/types/v2/spot.go (3)
204-219: LGTM on VWAP calculation logic.The
ApplyExecutionmethod correctly handles nil receivers by creating a new instance, and guards against nil/zero quantity inputs. The weighted average calculation(oldPrice * oldQty + newPrice * newQty) / totalQtyis mathematically correct for VWAP.
93-124: LGTM on deposit delta handling.The
UpdateFromDepositDeltasmethod correctly handles:
- Positive change amounts being added to available balance
- Zero subaccount fallback to auction subaccount for fee recipients
- Proper application of trader and fee recipient deltas
126-190: LGTM on batch execution event construction.The function properly:
- Preallocates slices with appropriate capacity
- Skips zero-fill trades to avoid empty entries
- Handles self-relayed trade fee calculation correctly
- Returns nil event when no trades occurred
chain/exchange/types/v2/fee_discounts.go (5)
84-95: LGTM - ValidatorCache type and NewFeeDiscountConfig constructor.The defensive nil check for
stakingInfoand settingisQualified = falseis appropriate.
97-121: LGTM - FeeDiscountConfig struct and GetFeeDiscountRate method.Proper nil receiver check and correct
RLock/RUnlockpattern for concurrent access.
123-162: LGTM - IncrementAccountVolumeContribution implementation.Defensive check for negative amounts, proper mutex locking, and correct map update patterns.
164-217: LGTM - FeeDiscountStakingInfo struct and constructor.All maps and mutexes are properly initialized.
323-383: LGTM - Remaining getter/setter methods.Proper mutex usage throughout.
SetAccountTierInfo,SetNewAccountTierTTL,AddCheckpoint, andAddInvalidGrantall correctly use Lock/Unlock.injective_data/chain_messages_list.json (1)
128-128: LGTM - New Chainlink message type registration.The new
MsgRelayChainlinkPricesmessage type is correctly added and properly positioned alphabetically within the oracle module namespace. This aligns with the PR objective of adding Chainlink data streams oracle support.proto/injective/oracle/v1beta1/events.proto (1)
90-93: LGTM - New Chainlink data streams event.The
EventSetChainlinkDataStreamsPricesmessage follows the established pattern used by similar events (EventSetStorkPrices,EventSetPythPrices) and correctly references theChainlinkDataStreamsPriceStatetype from the importedoracle.proto.chain/exchange/types/msgs.go (1)
2099-2111: LGTM - Duplicate grantee validation.The implementation correctly detects and rejects duplicate grantees within a single
MsgAuthorizeStakeGrantsmessage. The error message includes the duplicate grantee address for debugging, and the validation is performed before the amount validation.chain/exchange/types/v2/trade.go (1)
1-23: LGTM - TradeFeeData struct and constructor.Clean implementation with proper initialization using
math.LegacyZeroDec(). The struct provides a clear model for trade fee-related data, and the constructor correctly accepts the discounted rate while initializing other fields to zero.chain/evm/precompiles/exchange/i_exchange_module.abigen.go (1)
1-2: Generated file - verify ABI source consistency.This is an auto-generated binding file. The ABI update on line 206 (changing
uint256toExchangeTypes.UFixed256x18for price, quantity, margin, and related fields) appears consistent with the PR's goal of updating proto definitions for chain v1.18.0.Ensure the Solidity interface source (
IExchangeModule.solor equivalent) was updated first, and this file was regenerated usingabigenrather than manually edited.Also applies to: 206-206
chain/exchange/types/v2/authz_exchange_generic.go (2)
115-118: Verify intended behavior when hold is missing from context.When
getHoldreturnsok = false(context key not set or value is nil), the method returnsAccept: false. This means authorization will be denied if the caller forgets to set the hold in context.Is this the intended behavior? Consider whether:
- Missing hold should be treated as zero spend (accept with unchanged limit), or
- Missing hold should explicitly reject (current behavior)
If the current strict behavior is intentional, consider adding a comment explaining why missing hold results in rejection.
110-131: LGTM - Clean spend limit enforcement logic.The refactored
Acceptmethod implements a clear flow:
- No-limit case returns early (line 111-113)
- Missing hold context rejects authorization (line 115-118)
- Single-pass validation and deduction of spend limits (line 120-130)
The logic correctly handles the case where a hold denom isn't in the spend limit (returns false since
AmountOfreturns zero for missing denoms).proto/injective/oracle/v1beta1/query.proto (1)
226-234: LGTM - New Chainlink query messages.The new request/response message definitions follow the established patterns in this file (empty request for retrieving all states, response with repeated price state field).
chain/exchange/types/codec.go (3)
18-20: LGTM - Oneof interface registration for Amino.Registering oneof interfaces (
isOpenNotionalCap_CapandisFullDerivativeMarket_Info) is necessary for proper Amino JSON serialization of protobuf oneof fields. This follows the correct pattern for handling oneofs in the Cosmos SDK legacy Amino codec.
41-45: LGTM - New message type registrations.The new messages (
MsgDecreasePositionMargin,MsgEmergencySettleMarket,MsgAuthorizeStakeGrants,MsgActivateStakeGrant) are correctly registered in both:
- Legacy Amino codec (lines 41-45) for JSON serialization
- Interface registry (lines 122-126) for protobuf/gRPC
Registration names follow the established
exchange/MsgXconvention.Also applies to: 122-126
92-98: LGTM - Oneof implementation registrations.Concrete oneof types are registered with clear, descriptive names that distinguish between the variants:
exchange/OpenNotionalCapUncappedandexchange/OpenNotionalCapCappedexchange/FullDerivativeMarketPerpetualInfoandexchange/FullDerivativeMarketFuturesInfoThis enables proper Amino serialization/deserialization of these polymorphic types.
proto/injective/exchange/v2/proposal.proto (1)
17-21: LGTM - Tri-state enum for optional updates.The
DisableMinimalProtocolFeeUpdateenum correctly implements a tri-state pattern:
NoUpdate (0): Don't modify the existing valueFalse (1): Set the flag to false (enable minimal protocol fee)True (2): Set the flag to true (disable minimal protocol fee)This is the appropriate pattern for optional fields in update proposals where you need to distinguish between "don't change" and "set to false".
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (2)
34-35: Generated ABI/Bin update – no manual review neededThese large ABI/Bin string literals are generated and appear consistent with the added Failure event bindings; no manual changes suggested.
608-741: Failure event bindings mirror existing iterator/watch patterns correctlyThe
FixedSupplyBankERC20FailureIterator,FixedSupplyBankERC20Failurestruct, and theFilterFailure/WatchFailure/ParseFailuremethods follow the same patterns as the existing Approval/Transfer bindings and correctly useFilterLogs/WatchLogswithout indexed topics. Looks good.proto/injective/oracle/v1beta1/tx.proto (1)
46-50: Chainlink RPC wiring matches existing oracle Msg patterns
RelayChainlinkPricesis added consistently with other RPCs (Pyth, Stork, etc.) and uses the expected request/response message types. No issues here.chain/oracle/types/msgs.go (1)
27-27: Chainlink relay type and sdk.Msg registration are consistent
TypeMsgRelayChainlinkPricesand the_ sdk.Msg = &MsgRelayChainlinkPrices{}registration align with the existing oracle message conventions.Also applies to: 39-39
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (2)
34-35: Generated ABI/Bin update – treated as codegenThe updated ABI/Bin literals reflect the new
Failureevent; assuming these came from regenerating bindings, there’s nothing to adjust manually.
744-877: MintBurn Failure event bindings are correct and consistentThe
MintBurnBankERC20FailureIterator,MintBurnBankERC20Failurestruct, and associatedFilterFailure/WatchFailure/ParseFailuremethods follow the established iterator and subscription patterns and correctly omit indexed topics for this event. All good.chain/exchange/types/v2/msgs.go (2)
346-360: Stork timestamp comment accurately documents existing behaviourThe new comment about converting timestamps to nanoseconds matches the surrounding code and improves readability without changing semantics.
2227-2243: Duplicate grantee detection in MsgAuthorizeStakeGrantsThe new
seenGranteesmap cleanly prevents duplicateGranteeentries within a singleMsgAuthorizeStakeGrants, using validated bech32 strings as keys and returningErrInvalidStakeGranton duplicates. This is a sensible tightening of validation with minimal overhead.chain/exchange/types/v2/codec.go (2)
38-40: LGTM! Oneof interface and concrete type registrations are properly structured.The registration of oneof interfaces (
isOpenNotionalCap_Cap,isFullDerivativeMarket_Info) and their concrete implementations (OpenNotionalCap_Uncapped,OpenNotionalCap_Capped,FullDerivativeMarket_PerpetualInfo,FullDerivativeMarket_FuturesInfo) follows the correct pattern for Amino codec compatibility with protobuf oneof fields.Also applies to: 120-127
61-77: LGTM! New message types are properly registered.The new messages (
MsgDecreasePositionMargin,MsgEmergencySettleMarket,MsgAuthorizeStakeGrants,MsgActivateStakeGrant) are correctly registered in bothRegisterLegacyAminoCodecandRegisterInterfaces, ensuring proper serialization and interface compliance.Also applies to: 151-167
chain/exchange/types/v2/market.go (3)
107-110: LGTM! Maturation start condition correctly updated.The
IsStartingMaturationmethod now properly checksExpirationTwapStartBaseCumulativePriceto determine if maturation is starting, which aligns with the TWAP-based calculation approach for expiry futures markets.
371-394: LGTM! Market solvency and balance delta utilities are well-implemented.
IsMarketSolventprovides a clear check for market fund adequacy.GetMarketBalanceDeltacorrectly handles the reduce-only case by adding the trade fee back before computing the delta, as reduce-only orders have fees deducted from payout rather than held separately.
12-16: LGTM! NewMarketIDQuoteDenomMakerFeestruct is well-defined.The struct provides a clean grouping of market identification and maker fee information for fee-related operations.
chain/exchange/types/v2/proposal.go (4)
771-772: LGTM! Chainlink Data Streams oracle type support added.The addition of
oracletypes.OracleType_ChainlinkDataStreamsto the valid oracle types inOracleParams.ValidateBasic()enables support for the new Chainlink Data Streams oracle, which is the core objective of this PR.
1894-1903: LGTM!AdminInfo.ValidateBasic()implementation is correct.The validation properly:
- Validates the admin address using
sdk.AccAddressFromBech32only when non-empty- Checks
AdminPermissionsagainsttypes.MaxPermto ensure valid permission flags- Uses appropriate error types from the exchange module
452-456: LGTM! ConsistentAdminInfo.ValidateBasic()integration across proposals.The
AdminInfovalidation is now consistently applied acrossSpotMarketLaunchProposal,PerpetualMarketLaunchProposal, andExpiryFuturesMarketLaunchProposal, ensuring proper admin address and permission validation when provided.Also applies to: 918-922, 1031-1035
1686-1706: LGTM! Constructor properly initializes all fields.The
NewBinaryOptionsMarketParamUpdateProposalconstructor correctly accepts and assigns thehasDisabledMinimalProtocolFeeparameter along with all other fields.chain/exchange/types/v2/derivative.go (3)
14-41: LGTM! Order comparison logic is correct.
GetIsOrderLesscorrectly computes the distance from reference price for both buy and sell orders, and the sorting direction is properly controlled byisSortingFromWorstToBest.
749-765: LGTM! VWAP calculation handles edge cases properly.
ApplyExecutioncorrectly guards against nil values and zero quantity, preventing division by zero when computing the weighted average price.
978-1064: LGTM! Batch event construction with proper delta application.
ApplyDeltasAndGetDerivativeOrderBatchEventcorrectly applies deposit deltas, trading reward points, and constructs trade logs with proper fee calculations including self-relayed trade handling.injective_data/ofac.json (1)
36-36: Verify OFAC addresses against official SDN list.Three new Ethereum addresses have been added (lines 36, 62, 72). The addresses are validly formatted and the JSON structure is correct. However, confirm these addresses are from the official OFAC SDN (Specially Designated Nationals) list, as sanctions lists are compliance-critical.
Also applies to: 62-62, 72-72
| "exchange/v2/AtomicMarketOrderFeeMultiplierScheduleProposal", | ||
| nil, | ||
| ) | ||
| cdc.RegisterConcrete(&DenomMinNotionalProposal{}, "exchange/v2/DenomMinNotionalProposal", nil) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "codec.go" -path "*/exchange/types/v2/*" 2>/dev/null | head -20Repository: InjectiveLabs/sdk-go
Length of output: 98
🏁 Script executed:
cat -n ./chain/exchange/types/v2/codec.go | sed -n '1,220p'Repository: InjectiveLabs/sdk-go
Length of output: 13410
Register DenomMinNotionalProposal in govtypes.Content interface registry.
DenomMinNotionalProposal is registered in the Amino codec (line 104) but missing from the govtypes.Content interface implementations in RegisterInterfaces (lines 183-202). All other proposal types are consistently registered in both registries. Add it to maintain pattern consistency.
🤖 Prompt for AI Agents
In chain/exchange/types/v2/codec.go around line 104 and in the
RegisterInterfaces block at lines ~183-202, the DenomMinNotionalProposal type is
registered in the Amino codec but not added to the govtypes.Content interface
registrations; add DenomMinNotionalProposal to the RegisterInterfaces call so it
implements govtypes.Content alongside the other proposal types (place it with
the other proposal registrations in the same slice/map used for
govtypes.Content).
| notional := order.Fillable.Mul(order.Price) | ||
| fee := notional.Mul(positiveMakerFeePart) | ||
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | ||
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) |
There was a problem hiding this comment.
Potential division by zero when order.Quantity is zero.
If order.Quantity is zero, the calculation order.Margin.Mul(order.Fillable).Quo(order.Quantity) will panic. Consider adding a guard or ensuring orders with zero quantity are filtered upstream.
🔎 Proposed fix
+ if order.Quantity.IsZero() {
+ continue
+ }
+
notional := order.Fillable.Mul(order.Price)
fee := notional.Mul(positiveMakerFeePart)
remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| notional := order.Fillable.Mul(order.Price) | |
| fee := notional.Mul(positiveMakerFeePart) | |
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | |
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) | |
| if order.Quantity.IsZero() { | |
| continue | |
| } | |
| notional := order.Fillable.Mul(order.Price) | |
| fee := notional.Mul(positiveMakerFeePart) | |
| remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) | |
| cumulativeQuoteAmount = cumulativeQuoteAmount.Add(remainingMargin).Add(fee) |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/derivative.go around lines 77-80, the expression
order.Margin.Mul(order.Fillable).Quo(order.Quantity) can panic if order.Quantity
is zero; add a guard before performing the division (e.g., if
order.Quantity.IsZero() then handle by skipping this order, setting
remainingMargin to zero, or returning an error), or validate/filter out
zero-quantity orders upstream and document the assumption; ensure the chosen
behavior is explicit and covered by a unit test.
| // ComputeSyntheticVwapUnitDelta returns (price - markPrice) / markPrice | ||
| func (p *DerivativeVwapInfo) ComputeSyntheticVwapUnitDelta(marketID common.Hash) math.LegacyDec { | ||
| vwapInfo := p.PerpetualVwapInfo[marketID] | ||
| return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) | ||
| } |
There was a problem hiding this comment.
Potential division by zero in ComputeSyntheticVwapUnitDelta.
If MarkPrice is nil or zero, the division vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) will panic. Additionally, if the marketID is not found in PerpetualVwapInfo, dereferencing vwapInfo will cause a nil pointer dereference.
🔎 Proposed fix with nil and zero checks
func (p *DerivativeVwapInfo) ComputeSyntheticVwapUnitDelta(marketID common.Hash) math.LegacyDec {
vwapInfo := p.PerpetualVwapInfo[marketID]
+ if vwapInfo == nil || vwapInfo.MarkPrice == nil || vwapInfo.MarkPrice.IsZero() {
+ return math.LegacyZeroDec()
+ }
return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice)
}| func (info *FeeDiscountStakingInfo) GetSortedNewFeeDiscountAccountTiers() []*AccountTierTTL { | ||
| accountTiers := make([]*AccountTierTTL, 0, len(info.AccountFeeTiers)) | ||
| info.NewAccountsMux.RLock() | ||
| for k, v := range info.NewAccounts { | ||
| accountTiers = append(accountTiers, &AccountTierTTL{ | ||
| Account: sdk.AccAddress([]byte(string(k[:]))), | ||
| TierTTL: v, | ||
| }) | ||
| } | ||
| info.NewAccountsMux.RUnlock() | ||
| sort.SliceStable(accountTiers, func(i, j int) bool { | ||
| return bytes.Compare(accountTiers[i].Account.Bytes(), accountTiers[j].Account.Bytes()) < 0 | ||
| }) | ||
| return accountTiers | ||
| } |
There was a problem hiding this comment.
Incorrect slice capacity pre-allocation.
Line 241 uses len(info.AccountFeeTiers) as capacity, but the loop iterates over info.NewAccounts. This may cause unnecessary reallocations if the maps have different sizes.
🔎 Proposed fix
func (info *FeeDiscountStakingInfo) GetSortedNewFeeDiscountAccountTiers() []*AccountTierTTL {
- accountTiers := make([]*AccountTierTTL, 0, len(info.AccountFeeTiers))
+ accountTiers := make([]*AccountTierTTL, 0, len(info.NewAccounts))
info.NewAccountsMux.RLock()
for k, v := range info.NewAccounts {🤖 Prompt for AI Agents
In chain/exchange/types/v2/fee_discounts.go around lines 240 to 254, the slice
capacity is preallocated with len(info.AccountFeeTiers) but the loop iterates
over info.NewAccounts; change the capacity to len(info.NewAccounts) (or 0 if you
prefer letting append grow) so the slice is pre-sized correctly, keep the
lock/unlock and rest of logic unchanged.
| func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | ||
| accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers)) | ||
| info.AccountVolumesMux.RLock() | ||
| for k, v := range info.AccountVolumeContributions { | ||
| accountContributions = append(accountContributions, &AccountContribution{ | ||
| // use copy of value in closure, since the memory is not copied, it's reused. | ||
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | ||
| Account: sdk.AccAddress([]byte(string(k[:]))), | ||
| Amount: v, | ||
| }) | ||
| } | ||
| info.AccountVolumesMux.RUnlock() | ||
| sort.SliceStable(accountContributions, func(i, j int) bool { | ||
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | ||
| }) | ||
| return accountContributions | ||
| } |
There was a problem hiding this comment.
Incorrect slice capacity pre-allocation.
Line 257 uses len(info.AccountFeeTiers) as capacity, but the loop iterates over info.AccountVolumeContributions.
🔎 Proposed fix
func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution {
- accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers))
+ accountContributions := make([]*AccountContribution, 0, len(info.AccountVolumeContributions))
info.AccountVolumesMux.RLock()
for k, v := range info.AccountVolumeContributions {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | |
| accountContributions := make([]*AccountContribution, 0, len(info.AccountFeeTiers)) | |
| info.AccountVolumesMux.RLock() | |
| for k, v := range info.AccountVolumeContributions { | |
| accountContributions = append(accountContributions, &AccountContribution{ | |
| // use copy of value in closure, since the memory is not copied, it's reused. | |
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | |
| Account: sdk.AccAddress([]byte(string(k[:]))), | |
| Amount: v, | |
| }) | |
| } | |
| info.AccountVolumesMux.RUnlock() | |
| sort.SliceStable(accountContributions, func(i, j int) bool { | |
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | |
| }) | |
| return accountContributions | |
| } | |
| func (info *FeeDiscountStakingInfo) GetSortedAccountVolumeContributions() []*AccountContribution { | |
| accountContributions := make([]*AccountContribution, 0, len(info.AccountVolumeContributions)) | |
| info.AccountVolumesMux.RLock() | |
| for k, v := range info.AccountVolumeContributions { | |
| accountContributions = append(accountContributions, &AccountContribution{ | |
| // use copy of value in closure, since the memory is not copied, it's reused. | |
| // So if your closure captures it, instead of copying via call args, you'll get same index in all goroutines | |
| Account: sdk.AccAddress([]byte(string(k[:]))), | |
| Amount: v, | |
| }) | |
| } | |
| info.AccountVolumesMux.RUnlock() | |
| sort.SliceStable(accountContributions, func(i, j int) bool { | |
| return bytes.Compare(accountContributions[i].Account.Bytes(), accountContributions[j].Account.Bytes()) < 0 | |
| }) | |
| return accountContributions | |
| } |
🤖 Prompt for AI Agents
In chain/exchange/types/v2/fee_discounts.go around lines 256-272, the slice is
preallocated with len(info.AccountFeeTiers) but the loop iterates over
info.AccountVolumeContributions; change the capacity to
len(info.AccountVolumeContributions) when calling make so the slice is correctly
sized for the data being appended.
| rpc SubaccountPositions(QuerySubaccountPositionsRequest) | ||
| returns (QuerySubaccountPositionsResponse) { | ||
| option (google.api.http).get = | ||
| "/injective/exchange/v1beta1/positions/{subaccount_id}"; | ||
| "/injective/exchange/v1beta1/subaccount_positions/{subaccount_id}"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to the old HTTP path pattern
echo "=== Searching for old path pattern (positions/{subaccount_id}) ==="
rg "positions/\{subaccount_id\}" -C2
echo -e "\n=== Searching for alternative path patterns ==="
rg "positions.*subaccount_id" -C2
echo -e "\n=== Searching for SubaccountPositions references ==="
rg "SubaccountPositions" -C3
echo -e "\n=== Checking for HTTP path definitions in query.proto ==="
rg "google.api.http" proto/injective/exchange/v1beta1/query.proto -C2Repository: InjectiveLabs/sdk-go
Length of output: 50376
API endpoint path clarified, but breaking change for REST API clients.
The updated path /injective/exchange/v1beta1/subaccount_positions/{subaccount_id} is more explicit and aligns better with the SubaccountPositions method name, distinguishing it from the general Positions endpoint.
This is a breaking change for external REST API clients directly calling this endpoint. However, SDK users relying on the generated gRPC client method (FetchChainSubaccountPositions) are unaffected since they use the RPC layer, not hardcoded HTTP paths.
Ensure that REST API clients and downstream services relying on direct HTTP calls are updated with the new path.
🤖 Prompt for AI Agents
In proto/injective/exchange/v1beta1/query.proto around lines 259-263, the HTTP
path for SubaccountPositions was changed to
/injective/exchange/v1beta1/subaccount_positions/{subaccount_id}, which breaks
existing REST clients; to fix this without breaking clients, add an additional
HTTP binding that preserves the old path (using
google.api.http.additional_bindings) mapping the previous endpoint to the same
RPC, or if you intend to break compatibility, update the API docs/changelog and
notify downstreams and SDK maintainers to replace the old REST URL with the new
one.
| // Retrieves the state for all Chainlink Data Streamsprice feeds | ||
| rpc ChainlinkDataStreamsPriceStates( | ||
| QueryChainlinkDataStreamsPriceStatesRequest) | ||
| returns (QueryChainlinkDataStreamsPriceStatesResponse) { | ||
| option (google.api.http).get = | ||
| "/injective/oracle/v1beta1/chainlink_data_stream_price_states"; | ||
| } |
There was a problem hiding this comment.
Minor typo in comment and HTTP path inconsistency.
- Line 79: "Streamsprice" should be "Streams price" (missing space)
- HTTP path uses singular
chainlink_data_stream_price_stateswhile the RPC/message names use pluralChainlinkDataStreams- consider aligning for consistency (e.g.,chainlink_data_streams_price_states)
🔎 Suggested fix
- // Retrieves the state for all Chainlink Data Streamsprice feeds
+ // Retrieves the state for all Chainlink Data Streams price feeds
rpc ChainlinkDataStreamsPriceStates(
QueryChainlinkDataStreamsPriceStatesRequest)
returns (QueryChainlinkDataStreamsPriceStatesResponse) {
option (google.api.http).get =
- "/injective/oracle/v1beta1/chainlink_data_stream_price_states";
+ "/injective/oracle/v1beta1/chainlink_data_streams_price_states";
}🤖 Prompt for AI Agents
In proto/injective/oracle/v1beta1/query.proto around lines 79 to 85, fix the
comment typo by changing "Streamsprice" to "Streams price" and make the HTTP
path consistent with the RPC/message pluralization by renaming the option path
from "/injective/oracle/v1beta1/chainlink_data_stream_price_states" to
"/injective/oracle/v1beta1/chainlink_data_streams_price_states"; also search for
and update any other references to the old singular path to avoid mismatches.
…81. Includes support for the new Chainlink oracle messages.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@chain/exchange/types/wasm_trade_summary.go`:
- Around line 68-100: The validateTrades method in SyntheticTradeAction
currently only compares fields pairwise and skips running individual trade
validation; call each SyntheticTrade's Validate() for both userTrade and
contractTrade inside the loop (or before parity checks) and return a wrapped
ErrInvalidTrade that includes the index and whether the failing trade is a user
or contract trade when Validate() errors; ensure you reference the
SyntheticTrade.Validate method and the UserTrades/ContractTrades slices and keep
existing parity checks after successful per-trade validation.
In `@chain/oracle/types/msgs.go`:
- Around line 322-329: MsgRelayChainlinkPrices.ValidateBasic currently only
checks Sender; add light sanity checks on the reports to reject no-op/malformed
submissions: ensure msg.Reports is not nil/empty, iterate reports and reject if
any entry is nil or has missing/zero-required fields (e.g. empty feed/asset
identifier, roundId == 0, timestamp == 0, and any numeric price/answer/aggregate
fields that must be non-zero for a valid report), and return a descriptive
error; keep the existing sdk.AccAddressFromBech32(msg.Sender) check and return
early on failure.
- Around line 31-38: The MsgRelayChainlinkPrices message is not registered in
the codec/interface registry; update RegisterLegacyAminoCodec and
RegisterInterfaces to include MsgRelayChainlinkPrices alongside the other
MsgRelay* types so it can be (de)serialized. In RegisterLegacyAminoCodec add the
MsgRelayChainlinkPrices concrete type to the amino registration block (same
style as MsgRelayPythPrices/MsgRelayStorkPrices), and in RegisterInterfaces add
the MsgRelayChainlinkPrices type to the sdk.Msg interface registrations and to
the msg service descriptor registration so MsgRelayChainlinkPrices is
discoverable by the interface registry.
In `@chain/oracle/types/params.go`:
- Around line 55-58: The field AcceptUnverifiedChainlinkDataStreamsReports is
defaulting to true which permits unverified Chainlink data; confirm intended
behavior and if production/mainnet should require verification, change the
default in the params struct initializer (symbol:
AcceptUnverifiedChainlinkDataStreamsReports in params.go) to false, update any
configuration loading/flag parsing that sets or overrides this value
(constructor/validation functions that reference this field), and add/update
unit tests and docs to reflect the hardened default so behavior is explicit.
In `@Makefile`:
- Around line 6-7: The Makefile target clone-injective-core currently pins the
git clone to pre-release branch v1.18.0-alpha.3; either document this decision
or pin to a stable release: update the clone-injective-core target (the git
clone ... -b ... flag) to use v1.17.2 if you want stable compatibility, or add a
clear comment above the target explaining why v1.18.0-alpha.3 is required (e.g.,
network/feature dependency) and note that it is a pre-release so
reviewers/consumers are aware; make the change directly in the Makefile around
the clone-injective-core target and the git clone -b argument.
♻️ Duplicate comments (3)
chain/exchange/types/v2/codec.go (1)
205-224: RegisterDenomMinNotionalProposalasgovtypes.Content.It’s registered in Amino but still missing from the gov content interface registry, so decoding/dispatch can fail for governance proposals of this type.
🐛 Proposed fix
registry.RegisterImplementations( (*govtypes.Content)(nil), &ExchangeEnableProposal{}, &BatchExchangeModificationProposal{}, &SpotMarketParamUpdateProposal{}, &SpotMarketLaunchProposal{}, &PerpetualMarketLaunchProposal{}, &ExpiryFuturesMarketLaunchProposal{}, &DerivativeMarketParamUpdateProposal{}, &MarketForcedSettlementProposal{}, &UpdateAuctionExchangeTransferDenomDecimalsProposal{}, &TradingRewardCampaignLaunchProposal{}, &TradingRewardCampaignUpdateProposal{}, &TradingRewardPendingPointsUpdateProposal{}, &FeeDiscountProposal{}, &BatchCommunityPoolSpendProposal{}, &BinaryOptionsMarketParamUpdateProposal{}, &BinaryOptionsMarketLaunchProposal{}, &AtomicMarketOrderFeeMultiplierScheduleProposal{}, + &DenomMinNotionalProposal{}, )chain/exchange/types/v2/derivative.go (2)
44-85: Guard against zeroQuantitybefore division.
order.Margin.Mul(order.Fillable).Quo(order.Quantity)will panic whenQuantityis zero.🔒 Proposed fix
- remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity) + if order.Quantity.IsZero() { + continue + } + remainingMargin := order.Margin.Mul(order.Fillable).Quo(order.Quantity)
859-863: Guard against nil/zero mark price in synthetic VWAP delta.
vwapInfoorMarkPricecan be nil (or zero), which would panic on division.🔒 Proposed fix
func (p *DerivativeVwapInfo) ComputeSyntheticVwapUnitDelta(marketID common.Hash) math.LegacyDec { vwapInfo := p.PerpetualVwapInfo[marketID] - return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) + if vwapInfo == nil || vwapInfo.MarkPrice == nil || vwapInfo.MarkPrice.IsZero() { + return math.LegacyZeroDec() + } + return vwapInfo.VwapData.Price.Sub(*vwapInfo.MarkPrice).Quo(*vwapInfo.MarkPrice) }
🧹 Nitpick comments (6)
chain/exchange/types/wasm_trades.go (1)
77-79: Consider adding a nil guard for defensive safety.If
IsReduceOnly()is called on an unvalidatedSyntheticTradewhereMargin.IsNil()is true, callingIsZero()could panic. While the normal flow validates first viaValidate(), adding a nil check would make this method safer for standalone use.♻️ Suggested defensive implementation
func (t *SyntheticTrade) IsReduceOnly() bool { - return t.Margin.IsZero() + return !t.Margin.IsNil() && t.Margin.IsZero() }chain/exchange/types/wasm_trade_summary.go (1)
32-44: Consider adding a nil guard inUpdate()for defensive programming.Currently,
Marketsis always initialized beforeUpdate()is called viainitSummary()(line 62), so no panic risk exists in the current codebase. However, sinceSyntheticTradeActionSummaryis an exported type, adding a defensive nil check is good practice to protect against external misuse:if s.Markets == nil { s.Markets = make(map[common.Hash]bool) }Regarding the address-only validation (line 39): confirm whether comparing
addressdirectly is intentional, or if the fullSubaccountIDshould be validated for consistency.chain/exchange/types/v2/params.go (1)
360-367:IsHexAddressdoes not validate EIP-55 checksums—it accepts any valid 40-character hex string including incorrect checksums.
ethcommon.IsHexAddressonly checks for a valid0xprefix (optional), exactly 40 hex characters, and valid hex range; it returnstruefor all-lowercase, all-uppercase, and mixed-case addresses regardless of checksum correctness. If your policy requires EIP-55 checksum enforcement, usecommon.MixedcaseAddress(...).ValidChecksum()instead.chain/exchange/types/proposal.go (1)
1655-1665: Consider including the admin address in invalid-address errors (optional).Wrapping with
err.Error()drops the failing address; usinga.Adminimproves diagnostics.♻️ Suggested tweak
- if _, err := sdk.AccAddressFromBech32(a.Admin); err != nil { - return errors.Wrap(ErrInvalidAddress, err.Error()) - } + if _, err := sdk.AccAddressFromBech32(a.Admin); err != nil { + return errors.Wrap(ErrInvalidAddress, a.Admin) + }chain/exchange/types/v2/proposal.go (2)
277-278: Expose the minimal‑protocol‑fee flag in the derivative update constructor.
ValidateBasictreatsHasDisabledMinimalProtocolFeeas an update, butNewDerivativeMarketParamUpdateProposalstill can’t set it directly. Consider adding a constructor parameter for symmetry and API completeness.Also applies to: 545-546, 1746-1747
1893-1903: Consider including the admin address in invalid-address errors (optional).Wrapping with
err.Error()drops the failing address; usinga.Adminimproves diagnostics.♻️ Suggested tweak
- if _, err := sdk.AccAddressFromBech32(a.Admin); err != nil { - return errors.Wrap(types.ErrInvalidAddress, err.Error()) - } + if _, err := sdk.AccAddressFromBech32(a.Admin); err != nil { + return errors.Wrap(types.ErrInvalidAddress, a.Admin) + }
| func (a *SyntheticTradeAction) validateTrades() error { | ||
| if len(a.UserTrades) == 0 || len(a.ContractTrades) == 0 { | ||
| return errors.Wrapf(ErrInvalidTrade, "no trades in action") | ||
| } | ||
|
|
||
| if len(a.UserTrades) != len(a.ContractTrades) { | ||
| return errors.Wrapf( | ||
| ErrInvalidTrade, | ||
| "mismatched user and contract trades: %d vs %d", | ||
| len(a.UserTrades), | ||
| len(a.ContractTrades), | ||
| ) | ||
| } | ||
|
|
||
| for i, userTrade := range a.UserTrades { | ||
| contractTrade := a.ContractTrades[i] | ||
|
|
||
| if userTrade.MarketID != contractTrade.MarketID { | ||
| return errors.Wrapf(ErrInvalidTrade, "mismatched user and contract trade at index %d", i) | ||
| } | ||
| if userTrade.IsBuy == contractTrade.IsBuy { | ||
| return errors.Wrapf(ErrInvalidTrade, "mismatched user and contract trade at index %d", i) | ||
| } | ||
| if !userTrade.Quantity.Equal(contractTrade.Quantity) { | ||
| return errors.Wrapf(ErrInvalidTrade, "mismatched user and contract trade at index %d", i) | ||
| } | ||
| if !userTrade.Price.Equal(contractTrade.Price) { | ||
| return errors.Wrapf(ErrInvalidTrade, "mismatched user and contract trade at index %d", i) | ||
| } | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| func (a *SyntheticTradeAction) Summarize() (*SyntheticTradeActionSummary, error) { | ||
| summary := NewSyntheticTradeActionSummary() | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and explore the surrounding context
find . -name "wasm_trade_summary.go" -type fRepository: InjectiveLabs/sdk-go
Length of output: 184
🏁 Script executed:
# Look for trade type definitions and IsValid methods
rg -n "type.*Trade.*struct" --type=go -A 5Repository: InjectiveLabs/sdk-go
Length of output: 50376
🏁 Script executed:
# Check for IsValid methods on trade-related types
rg -n "func.*IsValid" --type=go -B 2 -A 3Repository: InjectiveLabs/sdk-go
Length of output: 3129
🏁 Script executed:
# Search for SyntheticTrade usage patterns
rg -n "SyntheticTrade" --type=go -B 2 -A 2 | head -100Repository: InjectiveLabs/sdk-go
Length of output: 5718
🏁 Script executed:
# Look at the specific file to understand the complete context
cat chain/exchange/types/wasm_trade_summary.go 2>/dev/null | head -150Repository: InjectiveLabs/sdk-go
Length of output: 3073
🏁 Script executed:
# Check if there's a previous version or git history that shows Validate() being called
cd chain/exchange/types && git log --oneline -n 20 wasm_trade_summary.go 2>/dev/null || echo "Git history not available"Repository: InjectiveLabs/sdk-go
Length of output: 135
🏁 Script executed:
# Verify that nil slice elements are possible by checking how trades are created/unmarshaled
rg -n "UserTrades\s*=" chain/exchange/types/ --type=go -B 2 -A 2Repository: InjectiveLabs/sdk-go
Length of output: 46
🏁 Script executed:
# Check how SyntheticTradeAction is instantiated/deserialized
rg -n "SyntheticTradeAction" chain/exchange/types/wasm_trades.go --type=go -A 5Repository: InjectiveLabs/sdk-go
Length of output: 425
Add per-trade validation in validateTrades.
The function only validates pairwise parity (market/buy/qty/price) and skips individual trade validation. The Validate() method on SyntheticTrade checks non-zero market IDs, valid subaccount IDs, and positive quantity/price—guards that should not be bypassed.
🛠️ Suggested fix
for i, userTrade := range a.UserTrades {
contractTrade := a.ContractTrades[i]
+ if err := userTrade.Validate(); err != nil {
+ return errors.Wrapf(ErrInvalidTrade, "invalid user trade at index %d: %v", i, err)
+ }
+ if err := contractTrade.Validate(); err != nil {
+ return errors.Wrapf(ErrInvalidTrade, "invalid contract trade at index %d: %v", i, err)
+ }
if userTrade.MarketID != contractTrade.MarketID {
return errors.Wrapf(ErrInvalidTrade, "mismatched user and contract trade at index %d", i)
}🤖 Prompt for AI Agents
In `@chain/exchange/types/wasm_trade_summary.go` around lines 68 - 100, The
validateTrades method in SyntheticTradeAction currently only compares fields
pairwise and skips running individual trade validation; call each
SyntheticTrade's Validate() for both userTrade and contractTrade inside the loop
(or before parity checks) and return a wrapped ErrInvalidTrade that includes the
index and whether the failing trade is a user or contract trade when Validate()
errors; ensure you reference the SyntheticTrade.Validate method and the
UserTrades/ContractTrades slices and keep existing parity checks after
successful per-trade validation.
| var ( | ||
| _ sdk.Msg = &MsgRelayPriceFeedPrice{} | ||
| _ sdk.Msg = &MsgRelayBandRates{} | ||
| _ sdk.Msg = &MsgRelayCoinbaseMessages{} | ||
| _ sdk.Msg = &MsgRequestBandIBCRates{} | ||
| _ sdk.Msg = &MsgRelayProviderPrices{} | ||
| _ sdk.Msg = &MsgRelayPythPrices{} | ||
| _ sdk.Msg = &MsgRelayStorkPrices{} | ||
| _ sdk.Msg = &MsgRelayChainlinkPrices{} | ||
| _ sdk.Msg = &MsgUpdateParams{} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for MsgRelayChainlinkPrices references
rg -n --type=go 'MsgRelayChainlinkPrices' -A 2Repository: InjectiveLabs/sdk-go
Length of output: 11855
🏁 Script executed:
#!/bin/bash
# Find codec and registry related files
find . -type f -name "*.go" | xargs grep -l "RegisterConcrete\|RegisterInterface" | head -20Repository: InjectiveLabs/sdk-go
Length of output: 668
🏁 Script executed:
#!/bin/bash
# Look for codec.go files and registry patterns
fd -e go -p ".*codec.*" | head -20Repository: InjectiveLabs/sdk-go
Length of output: 604
🏁 Script executed:
cat -n chain/oracle/types/codec.goRepository: InjectiveLabs/sdk-go
Length of output: 3880
🏁 Script executed:
#!/bin/bash
# Search for codec registration patterns in oracle module
rg -n "RegisterConcrete|RegisterInterface" chain/oracle/types/codec.go -A 1 -B 1Repository: InjectiveLabs/sdk-go
Length of output: 1504
Register MsgRelayChainlinkPrices in codec/interface registry.
MsgRelayChainlinkPrices is missing from both RegisterLegacyAminoCodec() (line 18-22) and RegisterInterfaces() (line 35-41) in chain/oracle/types/codec.go. Add the message type to both registration functions alongside the other message types to ensure proper transaction serialization and deserialization.
🤖 Prompt for AI Agents
In `@chain/oracle/types/msgs.go` around lines 31 - 38, The MsgRelayChainlinkPrices
message is not registered in the codec/interface registry; update
RegisterLegacyAminoCodec and RegisterInterfaces to include
MsgRelayChainlinkPrices alongside the other MsgRelay* types so it can be
(de)serialized. In RegisterLegacyAminoCodec add the MsgRelayChainlinkPrices
concrete type to the amino registration block (same style as
MsgRelayPythPrices/MsgRelayStorkPrices), and in RegisterInterfaces add the
MsgRelayChainlinkPrices type to the sdk.Msg interface registrations and to the
msg service descriptor registration so MsgRelayChainlinkPrices is discoverable
by the interface registry.
| // ValidateBasic implements the sdk.Msg interface for MsgRelayChainlinkPrices. | ||
| func (msg MsgRelayChainlinkPrices) ValidateBasic() error { | ||
| if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Add basic report sanity checks in ValidateBasic.
Currently empty or nil reports are accepted; this can allow no-op or malformed submissions.
✅ Suggested minimal validation
func (msg MsgRelayChainlinkPrices) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil {
return err
}
+
+ if len(msg.Reports) == 0 {
+ return errors.Wrap(sdkerrors.ErrInvalidRequest, "reports must not be empty")
+ }
+ for i, r := range msg.Reports {
+ if r == nil {
+ return errors.Wrapf(sdkerrors.ErrInvalidRequest, "reports[%d] is nil", i)
+ }
+ }
return nil
}🤖 Prompt for AI Agents
In `@chain/oracle/types/msgs.go` around lines 322 - 329,
MsgRelayChainlinkPrices.ValidateBasic currently only checks Sender; add light
sanity checks on the reports to reject no-op/malformed submissions: ensure
msg.Reports is not nil/empty, iterate reports and reject if any entry is nil or
has missing/zero-required fields (e.g. empty feed/asset identifier, roundId ==
0, timestamp == 0, and any numeric price/answer/aggregate fields that must be
non-zero for a valid report), and return a descriptive error; keep the existing
sdk.AccAddressFromBech32(msg.Sender) check and return early on failure.
| PythContract: "", | ||
| ChainlinkVerifierProxyContract: "", | ||
| AcceptUnverifiedChainlinkDataStreamsReports: true, | ||
| ChainlinkDataStreamsVerificationGasLimit: 500_000, |
There was a problem hiding this comment.
Confirm default “accept unverified” is intended.
AcceptUnverifiedChainlinkDataStreamsReports defaults to true, which could allow unverified price data. Please confirm this is the intended default for mainnet/production; otherwise consider defaulting to false.
#!/bin/bash
# Inspect how the unverified flag is used to gate verification.
rg -n --type=go 'AcceptUnverifiedChainlinkDataStreamsReports'🔧 Potential default hardening (if verification should be required)
- AcceptUnverifiedChainlinkDataStreamsReports: true,
+ AcceptUnverifiedChainlinkDataStreamsReports: false,🤖 Prompt for AI Agents
In `@chain/oracle/types/params.go` around lines 55 - 58, The field
AcceptUnverifiedChainlinkDataStreamsReports is defaulting to true which permits
unverified Chainlink data; confirm intended behavior and if production/mainnet
should require verification, change the default in the params struct initializer
(symbol: AcceptUnverifiedChainlinkDataStreamsReports in params.go) to false,
update any configuration loading/flag parsing that sets or overrides this value
(constructor/validation functions that reference this field), and add/update
unit tests and docs to reflect the hardened default so behavior is explicit.
| clone-injective-core: | ||
| git clone https://github.com/InjectiveLabs/injective-core.git -b v1.17.0 --depth 1 --single-branch | ||
| git clone https://github.com/InjectiveLabs/injective-core.git -b v1.18.0-alpha.3 --depth 1 --single-branch |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
injective-core latest stable release version
💡 Result:
The latest stable release of injective-core (Docker image injectivelabs/injective-core) is v1.17.2. (github.com)
Citations:
🏁 Script executed:
# Check if there's any documentation in the repo about versioning strategy
find . -type f \( -name "*.md" -o -name "*.txt" \) | head -20 | xargs grep -l "injective-core\|version" 2>/dev/null | head -5Repository: InjectiveLabs/sdk-go
Length of output: 184
🏁 Script executed:
# Check git history of the Makefile to see version progression
git log --oneline -p Makefile 2>/dev/null | grep -A 2 -B 2 "injective-core" | head -30Repository: InjectiveLabs/sdk-go
Length of output: 2080
Confirm alpha version pinning—either wait for stable v1.18.0 or document pre-release status.
The clone target uses v1.18.0-alpha.3, a pre-release version. The latest stable release is v1.17.2. If this alpha pin is intentional to support an upcoming chain release, add a comment in the Makefile explaining the reasoning. Otherwise, consider waiting for a stable v1.18.0 release or pin to v1.17.2 if compatibility is not required.
🤖 Prompt for AI Agents
In `@Makefile` around lines 6 - 7, The Makefile target clone-injective-core
currently pins the git clone to pre-release branch v1.18.0-alpha.3; either
document this decision or pin to a stable release: update the
clone-injective-core target (the git clone ... -b ... flag) to use v1.17.2 if
you want stable compatibility, or add a clear comment above the target
explaining why v1.18.0-alpha.3 is required (e.g., network/feature dependency)
and note that it is a pre-release so reviewers/consumers are aware; make the
change directly in the Makefile around the clone-injective-core target and the
git clone -b argument.
Solves C-655
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.